CI: fix template-injection findings in GitHub Actions workflows#22732
CI: fix template-injection findings in GitHub Actions workflows#22732jorgsowa wants to merge 1 commit into
Conversation
zizmor flagged several workflow steps that expand untrusted `${{ }}`
expressions directly into `run:` shell scripts. Route these values
through `env:` instead so they're substituted as shell variables
rather than interpolated into the script text.
TimWolla
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
I'm lukewarm on this entire change. The jobs run with minimal permissions (particularly on the pull_request trigger). Anyone who is able to inject untrusted values, will also be able to just execute arbitrary code by modifying the workflow and / or the Makefile.
| env: | ||
| EVENT_NAME: ${{ github.event_name }} | ||
| RUN_ATTEMPT: ${{ github.run_attempt }} | ||
| REF: ${{ github.event_name == 'pull_request' && github.ref || github.ref_name }} |
There was a problem hiding this comment.
Unrelated, but this could use the case() function.
|
I understand you and agree. The attack vector is very limited, but this hardening is becoming pretty standard. The end goal should be secure workflow and easy configuration. I tend to apply everything that zizmor warns about, to not care about customization and to avoid any divagation if the change is harmful or not when there are no strong opinions. I can compare it with the formatter, i.e. Prettier. Minimal configuration, code formatted and no one cares anymore about the formatting, because it's standarized. My recommandation is to apply full zizmor in CI, add dependabot/renovate and zizmor-action to the CI pipeline. I can do it in separate commits if you want, or you can close the PR. I can accept any decision. |
2 participants
zizmorflagged several workflow steps that expand untrusted${{ }}expressions directly intorun:shell scripts. Route these values throughenv:.I can also revert the change for safe GitHub-provided values, like
github.repository.